Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate OpenHands resolver into main repository #4964

Merged
merged 30 commits into from
Nov 14, 2024
Merged

Conversation

neubig
Copy link
Contributor

@neubig neubig commented Nov 13, 2024

This PR integrates the OpenHands resolver into the main repository to prevent updates to OpenHands from breaking the resolver and to make it easier to incorporate resolver actions into OpenHands server.

Changes:

  1. Move resolver code from openhands-resolver repo into openhands/resolver/
  2. Move tests into tests/unit/resolver/
  3. Update GitHub workflow to use integrated resolver
  4. Update imports to reflect new package structure

This will help solve https://github.com/All-Hands-AI/openhands-resolver/issues/155 by allowing us to incorporate the resolver actions into OpenHands server without spinning up a separate server.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:1f2f2c9-nikolaik   --name openhands-app-1f2f2c9   docker.all-hands.dev/all-hands-ai/openhands:1f2f2c9

openhands-agent and others added 5 commits November 13, 2024 14:01
1. Move resolver code from openhands-resolver repo into openhands/resolver/
2. Move tests into tests/unit/resolver/
3. Update GitHub workflow to use integrated resolver
4. Update imports to reflect new package structure
@neubig neubig added the fix-me Attempt to fix this issue with OpenHands label Nov 13, 2024
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@neubig neubig removed the fix-me Attempt to fix this issue with OpenHands label Nov 13, 2024
Copy link
Contributor

@malhotra5 malhotra5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get to take a SUPERR close look at this but I tested this locally and it worked great!

Feedback mainly addresses migrating occurrences of openhands-resolver to openhands-ai

Also I think we're missing changes to the Openhands docs that point to the resolver READMEs, but I'm OK if that's done in a separate PR.

Copy link
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@neubig neubig merged commit a753bab into main Nov 14, 2024
18 checks passed
@neubig neubig deleted the integrate-resolver branch November 14, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants